Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename via_hub to via_device #24360

Merged
merged 2 commits into from Jun 10, 2019
Merged

Conversation

Swamp-Ig
Copy link
Contributor

@Swamp-Ig Swamp-Ig commented Jun 7, 2019

Breaking Change:

The breaking change is that via_hub is renamed to via_device. Since custom components can't access this at this stage it should in theory not be an issue.

Description:

As discussed in the arch issue, via_hub doesn't make that much sense as a name anymore since it's not strictly speaking actually a hub. This renames the variable.

Related issue (if applicable): fixes home-assistant/architecture#212

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

If the code does not interact with devices:

  • Tests have been added modified to verify that the new code works.

vol.Optional(CONF_VIA_HUB): cv.string,
}), validate_device_has_at_least_one_identifier)
MQTT_ENTITY_DEVICE_INFO_SCHEMA = vol.All(
cv.deprecated(CONF_DEPRECATED_VIA_HUB, CONF_VIA_DEVICE),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deprecate the old config option.

hub_device_id=device.get('hub_device_id'),
# renamed in 0.95
via_device_id=(
device.get('via_device_id')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This automatically upgrades the existing device registry on loading. Alternately we could do an update script instead, what's better here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a lot smaller than adding a script 👍 . Just make renamed -> Renamed

@rytilahti
Copy link
Member

Can you link some information about this one? Like what is its purpose (is it just metadata?), how is it used in homessistant etc. But when considering renaming it, wouldn't parent_device be a more neutral choice?

@Swamp-Ig
Copy link
Contributor Author

Swamp-Ig commented Jun 8, 2019

@rytilahti all discussed in the linked arch issue.

@rytilahti
Copy link
Member

Sorry, my bad. But the naming was not discussed in that issue that much. How does someone interpret via_device? Should it be interpreted as "controlled via device"? If so, wouldn't control_via_device be a better naming for this? I would prefer to have it explicitly stated to make it easy to interpret, that's all...

@Swamp-Ig
Copy link
Contributor Author

Swamp-Ig commented Jun 8, 2019

@rytilahti Read the last few comments on that issue 😁

@Swamp-Ig Swamp-Ig marked this pull request as ready for review June 8, 2019 08:57
@Swamp-Ig
Copy link
Contributor Author

Swamp-Ig commented Jun 8, 2019

OK have tested this now along with the front-end PR and I'm reasonably happy. The only things to resolve are what to do about upgrading the registry.

Copy link
Member

@Kane610 Kane610 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

@balloob balloob merged commit 84e6813 into home-assistant:dev Jun 10, 2019
@balloob
Copy link
Member

balloob commented Jun 10, 2019

Please don't forget to update the dev docs.

@Swamp-Ig
Copy link
Contributor Author

@balloob the front end PR home-assistant/frontend#3254 will need to be merged too to go along with it. Working on a dev doc PR right now. I grepped ha.io but not dev so didn't find it!

@Swamp-Ig
Copy link
Contributor Author

balloob pushed a commit to home-assistant/developers.home-assistant that referenced this pull request Jun 11, 2019
@Swamp-Ig Swamp-Ig deleted the via-hub-rename branch June 16, 2019 01:33
@balloob balloob mentioned this pull request Jun 26, 2019
alandtse pushed a commit to alandtse/home-assistant that referenced this pull request Oct 12, 2019
* Rename via_hub to via_device

* Fixed registry interactions
@bdraco bdraco mentioned this pull request Feb 1, 2021
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

split devices / config entries override area of device
5 participants